[Enhancement]Align peerconnection preparation timeout with the clients#1085
Merged
ipavlidakis merged 3 commits intodevelopfrom Mar 17, 2026
Conversation
📝 WalkthroughWalkthroughThis PR refactors WebRTC join flow telemetry handling by removing the timeout parameter from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Sources/StreamVideo/WebRTC/v2/StateMachine/Components/JoinedStateTelemetryReporter.swift`:
- Around line 42-45: In JoinedStateTelemetryReporter.swift the switch's .fast
branch shadows the outer reconnection variable and builds a new
Stream_Video_Sfu_Signal_Reconnection without setting timeSeconds, causing zero
durations to be reported; change the .fast case to reuse the existing
reconnection variable (do not redeclare it), set reconnection.strategy = .fast
(keeping reconnection.timeSeconds already assigned) and return
.reconnection(reconnection) so the real duration is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fe9e89f-f8e5-432d-b9a6-e19ef6cecc10
📒 Files selected for processing (12)
Sources/StreamVideo/WebRTC/v2/Policies/JoinPolicy/WebRTCJoinPolicy.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Components/JoinedStateTelemetryReporter.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Joining.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+PeerConnectionPreparing.swiftSources/StreamVideo/WebRTC/v2/WebRTCConfiguration.swiftSources/StreamVideoSwiftUI/CallViewModel.swiftStreamVideoSwiftUITests/CallViewModel_Tests.swiftStreamVideoTests/Call/Call_Tests.swiftStreamVideoTests/CallStateMachine/CallStateMachine/Stages/CallStateMachine_JoiningStageTests.swiftStreamVideoTests/Controllers/CallController_Tests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_JoiningStageTests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_PeerConnectionPreparingStageTests.swift
Comment on lines
+42
to
+45
| case .fast: | ||
| var reconnection = Stream_Video_Sfu_Signal_Reconnection() | ||
| reconnection.strategy = .fast | ||
| return .reconnection(reconnection) |
There was a problem hiding this comment.
Bug: .fast case creates a new reconnection without timeSeconds.
The .fast case shadows the outer reconnection variable and creates a new one without setting timeSeconds. This means fast reconnection telemetry will report timeSeconds = 0 instead of the actual duration, unlike .rejoin and .migrate which correctly reuse the outer variable.
🐛 Proposed fix
case .fast:
- var reconnection = Stream_Video_Sfu_Signal_Reconnection()
+ reconnection.strategy = .fast
- reconnection.strategy = .fast
return .reconnection(reconnection)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Sources/StreamVideo/WebRTC/v2/StateMachine/Components/JoinedStateTelemetryReporter.swift`
around lines 42 - 45, In JoinedStateTelemetryReporter.swift the switch's .fast
branch shadows the outer reconnection variable and builds a new
Stream_Video_Sfu_Signal_Reconnection without setting timeSeconds, causing zero
durations to be reported; change the .fast case to reuse the existing
reconnection variable (do not redeclare it), set reconnection.strategy = .fast
(keeping reconnection.timeSeconds already assigned) and return
.reconnection(reconnection) so the real duration is preserved.
Collaborator
SDK Size
|
Collaborator
StreamVideo XCSize
|
Collaborator
StreamVideoSwiftUI XCSize
|
Public Interface public enum WebRTCJoinPolicy: Sendable
- case peerConnectionReadinessAware(timeout: TimeInterval)
+ case peerConnectionReadinessAware |
martinmitrevski
approved these changes
Mar 17, 2026
|
ipavlidakis
deleted the
iliaspavlidakis/align-peerconnection-preparation-timeout-with-the-clients
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Goal
Align with the other clients on the timeout that we wait the peerconnections to connect.
☑️ Contributor Checklist
Summary by CodeRabbit
New Features
Improvements